-
Notifications
You must be signed in to change notification settings - Fork 2
tools docker #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
tools docker #34
Conversation
| docker build \ | ||
| --build-arg SISDK_Tag="$SISDK_Tag" \ | ||
| --build-arg WiFI_SDK_Tag="$WiFI_SDK_Tag" \ | ||
| -f docker/Dockerfile \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ln -s "$CONAN_PATH" "$INSTALL_PATH/conan" \ | ||
| else \ | ||
| echo "Error: conan not found at $CONAN_PATH" \ | ||
| fi \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.github/workflows/docker-publish.yml
Outdated
| if: ${{ github.event.inputs.tools == false }} | ||
| run: | | ||
| chmod +x docker/sdks/build.sh | ||
| ./docker/sdks/build.sh --tag "$VERSION" --push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ./docker/build.sh --tag "$VERSION" | ||
| fi | ||
| chmod +x docker/tools/build.sh | ||
| ./docker/tools/build.sh --tag "25Q4-Tools" --push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ./docker/build.sh --tag "$VERSION" | ||
| fi | ||
| chmod +x docker/tools/build.sh | ||
| ./docker/tools/build.sh --tag "25Q4-Tools" --push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Build Workflow Ignores Config, Incomplete Images.
The workflow unconditionally builds only the tools image with a hardcoded tag, ignoring both the push_images input parameter and the VERSION environment variable set in the previous step. This prevents building SDK images and makes the conditional version parsing logic unused.
| if [[ "${{ github.event.inputs.tools }}" == "false" ]]; then | ||
| ./docker/sdks/build.sh --tag "$VERSION" | ||
| else | ||
| ./docker/tools/build.sh --tag "$VERSION" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| VERSION="SiSDK${SISDK_Tag}_WiFi_SDK${WiFI_SDK_Tag}" | ||
| echo "VERSION=$VERSION" >> $GITHUB_ENV | ||
| echo "Parsed VERSION: $VERSION" | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Version Mismatch: Hardcoded Tag Overrides Computation
The VERSION environment variable is set conditionally but never used. The subsequent build step at line 55 uses a hardcoded tag "25Q4-Tools" instead of the computed VERSION, making this entire step pointless and wasting CI resources parsing the version file.
| if [[ "${{ github.event.inputs.tools }}" == "false" ]]; then | ||
| ./docker/sdks/build.sh --tag "$VERSION" | ||
| else | ||
| ./docker/tools/build.sh --tag "$VERSION" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Incorrect Syntax and Undefined Variable Break Script
The script uses GitHub Actions template syntax ${{ github.event.inputs.tools }} which only works within GitHub Actions workflow files. When executed as a standalone shell script, this becomes a literal string that never equals "false", causing the else branch to always execute. Additionally, the VERSION variable is referenced but never defined, causing the script to pass an empty tag argument.
Note
Introduce a new tools Docker image and refactor build workflow/scripts to toggle between SDKs and tools builds with new inputs.
workflow_dispatchinputs:push_images(default true) andtools(default true).VERSIONwhen not building tools; upgrade checkout toactions/checkout@v5.docker/tools/build.shtagged25Q4-Tools.docker/build.shto delegate todocker/sdks/build.shordocker/tools/build.shbased ontoolsinput.docker/sdks/build.shto build/pushghcr.io/siliconlabssoftware/matter_extension_dependenciesusing parsed SDK tags.docker/tools/Dockerfileto install SLT, conan engine, SLC CLI, Commander, and ARM GCC.docker/tools/build.shanddocker/tools/requirements.txt.Written by Cursor Bugbot for commit a2446dc. This will update automatically on new commits. Configure here.